Implement RichSuggestBox #3650

huynhsontung · 2021-01-03T00:52:18Z

Fixes

Closes #3649

Implement rich suggest text control as discussed in the issue.

PR Type

What kind of change does this PR introduce?

Feature

What is the current behavior?

None. The control did not exist before this PR.

What is the new behavior?

'RichSuggestBox' is now a usable control in the Microsoft.Toolkit.Uwp.UI.Controls namespace.

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Pull Request has been submitted to the documentation repository instructions. Link: Documentation for RichSuggestBox control MicrosoftDocs/WindowsCommunityToolkitDocs#558
Sample in sample app has been added / updated (for bug fixes / features)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

Other information

Convenient issue tracker

Limited styling options for tokens. Only text styling options are available.
Tokens are raw text and not UI elements. UI events (e.g. UIElement.PointerEntered) are not available. ~~This makes behaviors like showing flyout when hover mouse over the token impossible.~~
Tokens are always underlined.
Tokens cannot adapt to theme change dynamically. They will lose foreground color while having the same background color which will most likely introduce bad contrast.
Cannot disable spelling check on tokens (token text may have red underline).
~~Cannot use Tab to select a suggestion due to conflict with the accessibility feature.~~
Deleted tokens will be kept in an internal list until cleared programmatically to support Undo and Redo.

RichEditBox issues

Below are RichEditBox related issues encountered during the development of this control.

Getting Link property from a range might change the range.
Replacing a formatted link at the beginning of the document does not reset the formatting. The replaced text still has the same formatting.
Setting the Link property can sometimes fail but only when the target text is at the beginning of the document.
There is no event for when text is about to change. TextChanging and SelectionChanging are invoked after the text has changed.
Character format foreground color cannot display a range of colors (e.g. #F7FFB3)

ghost · 2021-01-03T00:52:23Z

Thanks huynhsontung for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

net-foundation-cla · 2021-01-03T00:52:33Z

All CLA requirements met.

net-foundation-cla · 2021-01-03T00:52:33Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ huynhsontung sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

michael-hawker · 2021-01-12T17:34:44Z

Thanks @huynhsontung for the PR! Can you please run the UpdateHeaders.bat or '.\build.ps1 -target=UpdateHeaders' and commit the changes? This should get to the next stage in the CI to ensure everything can build and setup a preview package folks can more easily test out.

I've put this in the 7.1 milestone for now, so it may take us a bit of time to get to reviewing in more depth.

michael-hawker · 2021-01-13T22:05:13Z

@huynhsontung I'm not sure what happened with your Update Headers but it touched like every file in the toolkit... Can you revert the head of your branch, re-commit only the required headers and force-push back? Thanks!

There's also some StyleCop issues to resolve, see: https://dev.azure.com/dotnet/WindowsCommunityToolkit/_build/results?buildId=44049&view=logs&j=4a47a688-b50e-58f6-4d2c-6d1b829fc8e2&t=503927fd-5045-588b-b5d5-40c5f89cda06&l=10755

huynhsontung · 2021-01-13T22:12:15Z

@michael-hawker I was wondering why the script changed every file in the repo. Does the script check for line ending? I might have auto CRLF misconfigured.

I will resolve all the StyleCop issues at once.

huynhsontung · 2021-01-21T23:13:03Z

@michael-hawker I have resolved all the build issues. I also added a sample page for easy testing. Let me know what you think.

Features still need to be worked on at the moment:

Clear tokens on copy/cut.
Floating suggestion dropdown instead of bottom-edge dropdown style like AutoSuggestBox.

Kyaa-dost · 2021-01-22T00:36:33Z

@huoyaoyuan I can see that this PR is still being worked on but I have tried testing the sample and I have noticed that whenever the text in the field is highlighted from right to left, part of the font box cuts off.

Again I know this might not be the final version but just wanted to provide some feedback before the PR is ready to be tested.

michael-hawker · 2021-01-22T00:52:46Z

Thanks @huynhsontung for fixing up the issues so a build passes the CI! It makes it a lot easier for folks to pull down and play with! We're still heads-down on wrapping our 7.0 release at the moment, so it might take me a bit of time to dive in for more detailed feedback. I'm excited to check this control out though! 🙂

In the meantime it seems @Kyaa-dost has been helping to check things out, so let us know when your remaining issues are cleared up and he can help validate them as well.

michael-hawker

@huynhsontung starting to get back into things now that we've shipped 7.0. Haven't dug too deep into this yet, but have a couple of quick initial comments. It also has to be moved to the Input folder/project now that we've split up the controls package (basically fast-forward to our current changes). Maybe easiest to rebase?

We should probably think a bit about how we could test this type of control as well. We'll be trying to figure out in the next couple of weeks how we manage the bar for tests and have different avenues based on that. Should know more soon to share, but just wanted to give you a heads up.

Excited to test this out more as it's certainly an experience commonly seen on the web and in many social apps these days! 🎉

Microsoft.Toolkit.Uwp.UI.Controls/RichSuggestBox/RichSuggestBox.cs

Microsoft.Toolkit.Uwp.UI.Controls/RichSuggestBox/RichSuggestBox.xaml

huynhsontung · 2021-04-20T22:58:26Z

@michael-hawker thank you for letting me know. As I have been incredibly busy lately, I won't be able to work on these changes for the next week or two. I will definitely look into it soon after 🚀

huynhsontung · 2021-05-04T17:22:42Z

@michael-hawker I have moved the control to the Input project. Following up on your comments, I unregistered all events before registering and referenced WinUI style directly instead of using a copy.

I also discovered some behaviors of RichEditBox that may interfere with how tokens work, like formatting after removing formatted text. I will look into it and find workarounds for unwanted behaviors.

Regarding testing, I have not had much experience with UI testing myself. Will be interesting to know more about how WCT tests UI.

huynhsontung · 2021-05-21T00:38:24Z

As partially outlined in the issue, there are some shortcomings with the current RichSuggestBox that I can't really find a solution for:

There seems to be no way to remove the token's underline.
No way to disable proofing check on the token text.
Cannot use Tab key to select a token, since it is conflicted with Windows' navigation. I opted to use Ctrl + Tab instead.

That being said, I am pretty happy with how RichSuggestBox turned out. I will be focusing on writing tests for the control from now on. The control does have a lot of edge case handling.

michael-hawker · 2021-06-01T16:59:16Z

@huynhsontung are the unit tests running locally for you? Seems like one of the TokenizingTextBox tests is suddenly failing in the CI here? Not sure if there's a naming conflict in the style somewhere for the RichSuggestBox? That's the only thing I could think of that could influence the other test?

Microsoft.Toolkit.Uwp.UI.Controls.Input/RichSuggestBox/RichSuggestBox.xaml

michael-hawker · 2021-06-17T20:42:46Z

@huynhsontung glad you figured out the issue. Let us know if you need some guidance on writing unit tests or UI Interaction tests (though we're waiting for #4007 to be merged to resolve some issues with that).

In the meantime, we'll take a look at the corner cases. They don't seem to be too monumental, but will try it out and provide more feedback.

Thanks!

michael-hawker · 2021-06-21T23:26:09Z

FYI @huynhsontung I have a PR open with info on our whole testing setup: CommunityToolkit/WindowsCommunityToolkit-wiki#33 let me know if you have any questions.

Looks like there's a merge conflict from something now, so it may be good to rebase the PR?

huynhsontung · 2021-08-22T22:07:38Z

I have opened a PR for a RichSuggestBox docs page. Please find it here MicrosoftDocs/WindowsCommunityToolkitDocs#558

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/samples.json

Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]>

michael-hawker · 2021-08-25T20:35:55Z

Last two remaining items I'm aware of are:

Question on XAML Root
Dispose vs. Cancel for cleaning up the CancellationTokenSource?

michael-hawker · 2021-08-26T05:03:24Z

Microsoft.Toolkit.Uwp.UI.Controls.Input/RichSuggestBox/RichSuggestBox.Helpers.cs

+                return !ControlHelpers.IsXamlRootAvailable || element.XamlRoot.IsHostVisible;
+            }
+
+            var toWindow = element.TransformToVisual(null);


Looked up the docs, so looks TransformToVisual(null) is doing what we want already (getting coordinates from the tree root). You should grab the bounds the same way you did in IsElementInsideWindow, checking Window.Current null above doesn't seem right?

We want the window position relative to the top left of the screen, not to the tree root. I don't know if I can get absolute screen coordinates without using Window.

edit: From Window class docs, Window.Current is set for UWP and null for Desktop apps. If Window.Current is null we can just assume the element is on-screen.

See the docs on the migration here.

I think the missing thing this guide doesn't show is how to get the proper X, Y coordinates as you're using that from the window bounds. @marb2000?

huynhsontung · 2021-08-26T10:01:25Z

WindowsCommunityToolkit/Microsoft.Toolkit.Uwp.UI.Controls.Input/RichSuggestBox/RichSuggestBox.Helpers.cs

Lines 21 to 29 in 3401db0

    
           private static bool IsElementOnScreen(FrameworkElement element, double offsetX = 0, double offsetY = 0) 
        
           { 
        
               // DisplayInformation only works in UWP. No alternative to get DisplayInformation.ScreenHeightInRawPixels 
        
               // Tracking issues: 
        
               // https://github.com/microsoft/WindowsAppSDK/issues/114 
        
               // https://github.com/microsoft/microsoft-ui-xaml/issues/4228 
        
               // TODO: Remove when DisplayInformation.ScreenHeightInRawPixels alternative is available 
        
               return true;

As previously discussed, I have disabled the element on screen check due to not being able to get the screen size in a desktop app (DisplayInformation only works in UWP). As a result, the suggestion popup no longer opens upward when RichSuggestBox moves too close to the bottom edge of the screen. The popup still opens upward when RichSuggestBox is too close to the bottom edge of the current window.

Tracking issues:
microsoft/WindowsAppSDK#114
microsoft/microsoft-ui-xaml#4228

ghost · 2021-08-26T21:21:19Z

Hello @michael-hawker!

Because this pull request has the auto merge :zap: label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

michael-hawker · 2021-08-26T23:26:59Z

Thanks so much @huynhsontung for creating this amazing control! It's super exciting to have this experience in the Toolkit now for everyone! 🎉🎉🎉

ghost requested review from michael-hawker, azchohfi and Kyaa-dost January 3, 2021 00:52

ghost requested a review from Rosuavio January 3, 2021 00:52

michael-hawker added this to the 7.1 milestone Jan 5, 2021

huynhsontung force-pushed the RichSuggestBox branch from 5ace970 to c7894bb Compare January 15, 2021 01:58

Kyaa-dost added feature 💡 controls 🎛️ sample app 🖼 need documentation 📃 labels Jan 22, 2021

michael-hawker reviewed Apr 20, 2021

View reviewed changes

Microsoft.Toolkit.Uwp.UI.Controls/RichSuggestBox/RichSuggestBox.cs Outdated Show resolved Hide resolved

Microsoft.Toolkit.Uwp.UI.Controls/RichSuggestBox/RichSuggestBox.xaml Outdated Show resolved Hide resolved

huynhsontung force-pushed the RichSuggestBox branch from 7c4c2af to 2f17dcc Compare May 1, 2021 06:47

michael-hawker linked an issue May 21, 2021 that may be closed by this pull request

[Feature] Raw text support for TokenizingTextBox #3486

Closed

michael-hawker mentioned this pull request May 25, 2021

[Future] 7.1 Release Plan #4045

Closed

27 tasks

huynhsontung commented Jun 2, 2021

View reviewed changes

Microsoft.Toolkit.Uwp.UI.Controls.Input/RichSuggestBox/RichSuggestBox.xaml Outdated Show resolved Hide resolved

huynhsontung added 4 commits August 20, 2021 01:02

add tests for Load and Clear

cddc5a6

minor fix for the Load test case

70c9ef0

SuggestionsRequested -> SuggestionRequested

3587c40

update sample code bind

73e5f15

huynhsontung mentioned this pull request Aug 21, 2021

Documentation for RichSuggestBox control MicrosoftDocs/WindowsCommunityToolkitDocs#558

Merged

5 tasks

huynhsontung added 2 commits August 21, 2021 17:26

improve some comments

9b34912

fix suggestions not showing if no SuggestionRequested handler

32c3743

huynhsontung added 2 commits August 22, 2021 22:42

add icon for sample

f5a97d4

improve sample icon

ab1d3b3

michael-hawker reviewed Aug 23, 2021

View reviewed changes

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/samples.json Outdated Show resolved Hide resolved

michael-hawker reviewed Aug 23, 2021

View reviewed changes

Microsoft.Toolkit.Uwp.SampleApp/SamplePages/samples.json Outdated Show resolved Hide resolved

huynhsontung and others added 2 commits August 23, 2021 00:26

update code url for sample

2e9b350

Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]>

update doc url for sample

7b7974b

Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]>

michael-hawker added the next preview ✈️ Label for marking what we want to include in the next preview release for developers to try. label Aug 24, 2021

dont use ApplicationView when XamlRoot is available

3d03ce7

michael-hawker reviewed Aug 26, 2021

View reviewed changes

michael-hawker mentioned this pull request Aug 26, 2021

Window.Current.Bounds contains more information about Position, not just Size MicrosoftDocs/windows-dev-docs#3319

Open

disable element on screen check

3401db0

Still apply Screen size algorithm for UWP and annotate calculation

4e327ed

michael-hawker approved these changes Aug 26, 2021

View reviewed changes

Provide better guards for UWP

46bd862

michael-hawker added the auto merge ⚡ label Aug 26, 2021

ghost merged commit fe9ac89 into CommunityToolkit:main Aug 26, 2021

michael-hawker mentioned this pull request Sep 1, 2021

🚨 7.1 Package Size Regressions #4218

Closed

5 tasks

Rosuavio mentioned this pull request Sep 3, 2021

UITests: Remove ref to incorrect version of winui #4226

Merged

11 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement RichSuggestBox #3650

Implement RichSuggestBox #3650

huynhsontung commented Jan 3, 2021 •

edited

Loading

ghost commented Jan 3, 2021

net-foundation-cla bot commented Jan 3, 2021 •

edited

Loading

net-foundation-cla bot commented Jan 3, 2021

michael-hawker commented Jan 12, 2021

michael-hawker commented Jan 13, 2021 •

edited

Loading

huynhsontung commented Jan 13, 2021 •

edited

Loading

huynhsontung commented Jan 21, 2021

Kyaa-dost commented Jan 22, 2021

michael-hawker commented Jan 22, 2021

michael-hawker left a comment

huynhsontung commented Apr 20, 2021

huynhsontung commented May 4, 2021

huynhsontung commented May 21, 2021

michael-hawker commented Jun 1, 2021

michael-hawker commented Jun 17, 2021

michael-hawker commented Jun 21, 2021 •

edited

Loading

huynhsontung commented Aug 22, 2021

michael-hawker commented Aug 25, 2021

michael-hawker Aug 26, 2021

huynhsontung Aug 26, 2021 •

edited

Loading

michael-hawker Aug 26, 2021

huynhsontung commented Aug 26, 2021

ghost commented Aug 26, 2021

michael-hawker commented Aug 26, 2021

Implement RichSuggestBox #3650

Implement RichSuggestBox #3650

Conversation

huynhsontung commented Jan 3, 2021 • edited Loading

Fixes

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

Convenient issue tracker

RichEditBox issues

ghost commented Jan 3, 2021

net-foundation-cla bot commented Jan 3, 2021 • edited Loading

net-foundation-cla bot commented Jan 3, 2021

michael-hawker commented Jan 12, 2021

michael-hawker commented Jan 13, 2021 • edited Loading

huynhsontung commented Jan 13, 2021 • edited Loading

huynhsontung commented Jan 21, 2021

Kyaa-dost commented Jan 22, 2021

michael-hawker commented Jan 22, 2021

michael-hawker left a comment

Choose a reason for hiding this comment

huynhsontung commented Apr 20, 2021

huynhsontung commented May 4, 2021

huynhsontung commented May 21, 2021

michael-hawker commented Jun 1, 2021

michael-hawker commented Jun 17, 2021

michael-hawker commented Jun 21, 2021 • edited Loading

huynhsontung commented Aug 22, 2021

michael-hawker commented Aug 25, 2021

michael-hawker Aug 26, 2021

Choose a reason for hiding this comment

huynhsontung Aug 26, 2021 • edited Loading

Choose a reason for hiding this comment

michael-hawker Aug 26, 2021

Choose a reason for hiding this comment

huynhsontung commented Aug 26, 2021

ghost commented Aug 26, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

michael-hawker commented Aug 26, 2021

huynhsontung commented Jan 3, 2021 •

edited

Loading

net-foundation-cla bot commented Jan 3, 2021 •

edited

Loading

michael-hawker commented Jan 13, 2021 •

edited

Loading

huynhsontung commented Jan 13, 2021 •

edited

Loading

michael-hawker commented Jun 21, 2021 •

edited

Loading

huynhsontung Aug 26, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.